Skip to content

No scripted fields for significant_terms#8723

Closed
spalger wants to merge 6 commits intoelastic:masterfrom
spalger:fix/significant-terms/no-scripted-fields
Closed

No scripted fields for significant_terms#8723
spalger wants to merge 6 commits intoelastic:masterfrom
spalger:fix/significant-terms/no-scripted-fields

Conversation

@spalger
Copy link
Contributor

@spalger spalger commented Oct 17, 2016

Builds on top of #8734

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting an undefined error on this line when I select an agg from the dropdown:

screen shot 2016-10-18 at 11 31 25 am

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for removing the conditional here? This seems to alter the flow of the loop so that if there are no indexedFields only the first param will ever be rendered, whereas before all params up to the field param would be rendered.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior you were seeing was a symptom of the error fixed in eaf5f1636fbebcca0b923de48576d84611ac8582.

This motivation for this change was preventing the aggParams directive from needing to know which params might need the indexedFields. As I say it though, I realize that every param for an aggConfig that has a field param is now getting indexedFields, because we are breaking out of the param and asking the agg for its fieldOptions. My mistake.

@spalger
Copy link
Contributor Author

spalger commented Oct 18, 2016

@Bargs I moved the meat of this over to #8734 so that we could talk about that outside of the scripted + significant issue.

@spalger spalger force-pushed the fix/significant-terms/no-scripted-fields branch 3 times, most recently from ac70791 to c5d6a91 Compare October 18, 2016 22:16
@spalger spalger force-pushed the fix/significant-terms/no-scripted-fields branch 2 times, most recently from f684a3b to 657e157 Compare October 18, 2016 22:27
@spalger spalger force-pushed the fix/significant-terms/no-scripted-fields branch from 657e157 to 0e11c22 Compare October 18, 2016 22:48
@spalger
Copy link
Contributor Author

spalger commented Oct 24, 2016

Merging into #8734

@spalger spalger closed this Oct 24, 2016
@spalger spalger deleted the fix/significant-terms/no-scripted-fields branch October 25, 2016 00:00
@spalger spalger restored the fix/significant-terms/no-scripted-fields branch November 27, 2016 09:57
@epixa epixa added v5.1.1 and removed v5.1.0 labels Dec 8, 2016
@spalger spalger deleted the fix/significant-terms/no-scripted-fields branch October 18, 2019 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants